Skip to content

fix(rpc): align eth_getBlockReceipts response with execution-apis spec#965

Open
XinyuCRO wants to merge 4 commits into
developfrom
fix/rpc/eth_getBlockReceipts
Open

fix(rpc): align eth_getBlockReceipts response with execution-apis spec#965
XinyuCRO wants to merge 4 commits into
developfrom
fix/rpc/eth_getBlockReceipts

Conversation

@XinyuCRO
Copy link
Copy Markdown

@XinyuCRO XinyuCRO commented May 26, 2026

No description provided.

Comment thread CHANGELOG.md Outdated
Signed-off-by: Xinyu <91446598+XinyuCRO@users.noreply.github.com>
@XinyuCRO
Copy link
Copy Markdown
Author

@claude review PR

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 26, 2026

Claude finished @XinyuCRO's task in 3m 24s —— View job


PR Review: fix(rpc): align eth_getBlockReceipts response with execution-apis spec

Tasks:

  • Read CLAUDE.md for repo guidelines
  • Analyze PR diff and changed files
  • Review code quality, correctness, and best practices
  • Check for breaking changes
  • Post inline comments for specific issues
  • Finalize review summary

Summary

This PR aligns eth_getBlockReceipts with the execution-apis spec by:

  1. Accepting BlockNumberOrHash instead of just BlockNumber (supports querying by block hash or tag like "safe")
  2. Adding effectiveGasPrice for all transaction types (previously only for DynamicFeeTxType)
  3. Adding blobGasUsed/blobGasPrice fields for BlobTxType transactions
  4. Fixing "safe" tag handling in BlockNumberOrHash.decodeFromString

No consensus/state-breaking changes detected. This is a JSON-RPC layer change only — it does not affect on-chain state, message signing, Amino JSON encoding, or module logic.


Assessment: Mostly Good ✅

The core approach is correct and well-structured. The interface change is clean and the new helper tendermintBlockByNumberOrHash is a sensible factoring. Tests are updated appropriately.


Issues Found

Minor / Nits

  • CHANGELOG.md — The author's own suggestion in the review correctly adds a missing space before fix(rpc):; that's the only formatting issue.
  • rpc/backend/blocks.go:138 — The silent fallback to EthLatestBlockNumber when both BlockHash and BlockNumber are nil is defensively fine, but undocumented. See inline comment.
  • rpc/backend/tx_info.go:477 — Blob gas fields are hardcoded to 0. Ethermint doesn't support EIP-4844, so this is safe today, but no comment explains why. See inline comment.
  • rpc/backend/tx_info.go:488 — Pre-existing dereference of GetEffectiveGasPrice return value without a nil check. Safe as long as go-ethereum's GasPrice() is non-nil, but worth noting. See inline comment.
  • rpc/backend/tx_info_test.go:935TestGetBlockReceipts_ByHash uses a zero hash that matches the hardcoded mock. Fragile test but works. See inline comment.

Spec compliance note: The execution-apis spec also requires effectiveGasPrice for EIP-2930 AccessList transactions (type: 0x1). The PR handles DynamicFeeTxType and BlobTxType but silently falls into the else branch (GetEffectiveGasPrice(nil), i.e. just gasPrice) for AccessListTxType. This is actually correct behavior — for EIP-2930 txs, there is no baseFee adjustment and gasPrice == effectiveGasPrice. Just worth confirming this is intentional. Fix or clarify →


No Breaking Changes

  • RPC interface signature changes (BlockNumberBlockNumberOrHash) are backward-compatible from a caller perspective: string block tags and hex block numbers continue to work; block-hash lookups are newly supported.
  • No changes to consensus, module state, Amino serialization, or tx structure.

| View PR branch

Comment thread CHANGELOG.md
Comment thread rpc/backend/tx_info.go
Comment thread rpc/backend/tx_info.go Outdated
Comment thread rpc/backend/blocks.go
Comment thread rpc/backend/tx_info_test.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

❌ Patch coverage is 62.96296% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.73%. Comparing base (f3a58a0) to head (bf08eb4).
⚠️ Report is 1 commits behind head on develop.

Files with missing lines Patch % Lines
rpc/backend/tx_info.go 50.00% 4 Missing and 2 partials ⚠️
rpc/namespaces/ethereum/eth/api.go 0.00% 3 Missing ⚠️
rpc/backend/blocks.go 90.90% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #965      +/-   ##
===========================================
+ Coverage    44.69%   44.73%   +0.04%     
===========================================
  Files          197      197              
  Lines        16930    16946      +16     
===========================================
+ Hits          7567     7581      +14     
  Misses        8555     8555              
- Partials       808      810       +2     
Files with missing lines Coverage Δ
rpc/backend/backend.go 77.77% <ø> (ø)
rpc/types/block.go 52.74% <100.00%> (ø)
rpc/backend/blocks.go 78.90% <90.90%> (+0.49%) ⬆️
rpc/namespaces/ethereum/eth/api.go 0.00% <0.00%> (ø)
rpc/backend/tx_info.go 60.64% <50.00%> (+0.29%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

XinyuCRO and others added 2 commits May 28, 2026 15:51
Signed-off-by: Xinyu <91446598+XinyuCRO@users.noreply.github.com>
@XinyuCRO XinyuCRO changed the title fix(rpc): align eth_getBlockReceipts response with execution-apis s… fix(rpc): align eth_getBlockReceipts response with execution-apis spec May 28, 2026
@XinyuCRO XinyuCRO marked this pull request as ready for review May 28, 2026 08:42
@XinyuCRO XinyuCRO requested a review from a team as a code owner May 28, 2026 08:42
@XinyuCRO XinyuCRO requested review from ApacCronos and randy-cro and removed request for a team May 28, 2026 08:42
Comment thread rpc/backend/tx_info.go
b.logger.Error("fetch basefee failed, node is pruned?", "height", res.Height, "error", err)
} else {
receipt["effectiveGasPrice"] = hexutil.Big(*ethMsg.GetEffectiveGasPrice(baseFee))
baseFee = nil
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When BaseFee() returns nil for a fee-market tx (whether due to pruning or fee market being disabled), the receipt should not contain effectiveGasPrice

@songgaoye
Copy link
Copy Markdown

LGTM, need to fix When BaseFee() returns nil for a fee-market tx (whether due to pruning or fee market being disabled), the receipt should not contain effectiveGasPrice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants